-
-
Notifications
You must be signed in to change notification settings - Fork 31
feat(tape-to-node-test): first draft
#296
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Bruno Rodrigues <[email protected]>
| const tapeImports = getNodeImportStatements(root, 'tape'); | ||
| const tapeRequires = getNodeRequireCalls(root, 'tape'); | ||
| const tapeImportCalls = getNodeImportCalls(root, 'tape'); | ||
|
|
||
| if ( | ||
| tapeImports.length === 0 && | ||
| tapeRequires.length === 0 && | ||
| tapeImportCalls.length === 0 | ||
| ) { | ||
| return null; | ||
| } | ||
|
|
||
| let testVarName = 'test'; | ||
|
|
||
| // 1. Replace imports | ||
| for (const imp of tapeImports) { | ||
| const defaultImport = imp.find({ | ||
| rule: { kind: 'import_clause', has: { kind: 'identifier' } }, | ||
| }); | ||
| if (defaultImport) { | ||
| const id = defaultImport.find({ rule: { kind: 'identifier' } }); | ||
| if (id) testVarName = id.text(); | ||
| edits.push( | ||
| imp.replace( | ||
| `import { test } from 'node:test';${EOL}import assert from 'node:assert';`, | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| for (const req of tapeRequires) { | ||
| const id = req.find({ | ||
| rule: { kind: 'identifier', inside: { kind: 'variable_declarator' } }, | ||
| }); | ||
| if (id) testVarName = id.text(); | ||
| const declaration = req | ||
| .ancestors() | ||
| .find( | ||
| (a) => | ||
| a.kind() === 'variable_declaration' || | ||
| a.kind() === 'lexical_declaration', | ||
| ); | ||
| if (declaration) { | ||
| edits.push( | ||
| declaration.replace( | ||
| `const { test } = require('node:test');${EOL}const assert = require('node:assert');`, | ||
| ), | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| for (const call of tapeImportCalls) { | ||
| const id = call.find({ | ||
| rule: { kind: 'identifier', inside: { kind: 'variable_declarator' } }, | ||
| }); | ||
| if (id) testVarName = id.text(); | ||
| const declaration = call | ||
| .ancestors() | ||
| .find( | ||
| (a) => | ||
| a.kind() === 'variable_declaration' || | ||
| a.kind() === 'lexical_declaration', | ||
| ); | ||
| if (declaration) { | ||
| edits.push( | ||
| declaration.replace( | ||
| `const { test } = await import('node:test');${EOL}const { default: assert } = await import('node:assert');`, | ||
| ), | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic is quite the same for the 3 difference scenarios, I think it can be simplified to code below
| const tapeImports = getNodeImportStatements(root, 'tape'); | |
| const tapeRequires = getNodeRequireCalls(root, 'tape'); | |
| const tapeImportCalls = getNodeImportCalls(root, 'tape'); | |
| if ( | |
| tapeImports.length === 0 && | |
| tapeRequires.length === 0 && | |
| tapeImportCalls.length === 0 | |
| ) { | |
| return null; | |
| } | |
| let testVarName = 'test'; | |
| // 1. Replace imports | |
| for (const imp of tapeImports) { | |
| const defaultImport = imp.find({ | |
| rule: { kind: 'import_clause', has: { kind: 'identifier' } }, | |
| }); | |
| if (defaultImport) { | |
| const id = defaultImport.find({ rule: { kind: 'identifier' } }); | |
| if (id) testVarName = id.text(); | |
| edits.push( | |
| imp.replace( | |
| `import { test } from 'node:test';${EOL}import assert from 'node:assert';`, | |
| ), | |
| ); | |
| } | |
| } | |
| for (const req of tapeRequires) { | |
| const id = req.find({ | |
| rule: { kind: 'identifier', inside: { kind: 'variable_declarator' } }, | |
| }); | |
| if (id) testVarName = id.text(); | |
| const declaration = req | |
| .ancestors() | |
| .find( | |
| (a) => | |
| a.kind() === 'variable_declaration' || | |
| a.kind() === 'lexical_declaration', | |
| ); | |
| if (declaration) { | |
| edits.push( | |
| declaration.replace( | |
| `const { test } = require('node:test');${EOL}const assert = require('node:assert');`, | |
| ), | |
| ); | |
| } | |
| } | |
| for (const call of tapeImportCalls) { | |
| const id = call.find({ | |
| rule: { kind: 'identifier', inside: { kind: 'variable_declarator' } }, | |
| }); | |
| if (id) testVarName = id.text(); | |
| const declaration = call | |
| .ancestors() | |
| .find( | |
| (a) => | |
| a.kind() === 'variable_declaration' || | |
| a.kind() === 'lexical_declaration', | |
| ); | |
| if (declaration) { | |
| edits.push( | |
| declaration.replace( | |
| `const { test } = await import('node:test');${EOL}const { default: assert } = await import('node:assert');`, | |
| ), | |
| ); | |
| } | |
| } | |
| const tapeImports = getNodeImportStatements(root, 'tape'); | |
| const tapeRequires = getNodeRequireCalls(root, 'tape'); | |
| const tapeImportCalls = getNodeImportCalls(root, 'tape'); | |
| const modDeps = [ | |
| ...tapeImports.map((node) => ({ | |
| node, | |
| import: `import { test } from 'node:test';${EOL}import assert from 'node:assert';`, | |
| })), | |
| ...tapeRequires.map((node) => ({ | |
| node, | |
| import: `const { test } = require('node:test');${EOL}const assert = require('node:assert');`, | |
| })), | |
| ...tapeImportCalls.map((node) => ({ | |
| node, | |
| import: `const { test } = await import('node:test');${EOL}const { default: assert } = await import('node:assert');`, | |
| })), | |
| ]; | |
| if (modDeps.length === 0) { | |
| return null; | |
| } | |
| let testVarName = 'test'; | |
| // 1. Replace imports | |
| for (const mod of modDeps) { | |
| if (mod.node.kind() === 'variable_declarator') { | |
| mod.node = mod.node.parent(); | |
| } | |
| const binding = mod.node.find({ | |
| rule: { | |
| any: [ | |
| { kind: 'identifier', inside: { kind: 'variable_declarator' } }, | |
| { | |
| kind: 'identifier', | |
| inside: { kind: 'import_clause', stopBy: 'end' }, | |
| }, | |
| ], | |
| }, | |
| }); | |
| if (binding) testVarName = binding.text(); | |
| edits.push(mod.node.replace(mod.import)); | |
| } |
ljharb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(btw the diff would read cleaner if it was "output" instead of "expected", because then "input" would be sorted before "output" instead of the reverse)
some of my comments apply on all the input/expected combos, so i'll post this now before being exhaustive
| // TODO: test.onFinish(() => { | ||
| // console.log('finished'); | ||
| // }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is a TODO for you, then this PR should probably still be a draft; if not, then it would be better to make it fail VERY LOUDLY so that users are aware they have to manually fix something. it's not ok to just silently elide their code :-)
| test('sync test with no args', async () => { | ||
| const a = 1; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a sync text needs to always be an async function in the test runner? O.o that seems like it introduces an unnecessary performance hit
| test("require test", async (t) => { | ||
| assert.strictEqual(1, 1); | ||
| // t.end(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this line should either be removed, or it should end up with whatever the node test runner has to say "this test is done and if you get more assertions later, it should error". (if it doesn't have that, then the node runner doesn't have sufficient features for a codemod from tape to be appropriate)
Related issue
Close #260